Fix HTML report missing detail data in change sections #24
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Replaced problematic {{ include }} directives with custom RenderChangeGroup function
- Added comprehensive test coverage for HTML formatter with all change types
- Fixed template context passing issue that caused empty sections
- All HTML formatter tests now pass (5/5)
Fixes #23
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a critical issue where HTML reports showed correct summary counts but displayed no detail data in the Breaking Changes, Added Items, and Removed Items sections. The root cause was Scriban's {{ include }} directive failing to properly pass template context to included templates.
- Replaced problematic
{{ include }}directives with a customRenderChangeGroupfunction that manually handles template rendering with proper context - Updated the main layout template to use the new custom function approach
- Added comprehensive test coverage with 5 test scenarios covering all change types
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| HtmlFormatterScribanTests.cs | New comprehensive test file covering all HTML formatter scenarios including added, removed, modified, and breaking changes |
| main-layout.scriban | Updated to use custom render_change_group function instead of problematic include directive |
| HtmlFormatterScriban.cs | Removed template caching, added custom RenderChangeGroup function for reliable template rendering with proper context |
| CompareCommand.cs | Minor command option alias addition for output file parameter |
Comments suppressed due to low confidence (1)
tests/DotNetApiDiff.Tests/Reporting/HtmlFormatterScribanTests.cs:147
- [nitpick] The test method name suggests it tests signature details, but based on the description this appears to be testing modified items. Consider renaming to
Format_WithModifiedItems_IncludesModifiedSectionto be consistent with other test names in the file.
public void Format_WithSignatures_IncludesSignatureDetails()
Signed-off-by: jbrinkman <joe.brinkman@improving.com>
Signed-off-by: jbrinkman <joe.brinkman@improving.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the issue where HTML reports showed correct counts but no detail data in the Breaking Changes, Added Items, and Removed Items sections.
Root Cause
The issue was caused by Scriban's
{{ include }}directive not properly passing template context to included templates. When the main layout template tried to include thechange-grouptemplate with section data, the context wasn't being passed correctly, resulting in empty template variables.Solution
{{ include }}directives with custom Scriban function: CreatedRenderChangeGroupcustom function that manually handles template rendering with proper contextmain-layout.scribanto use the new custom function instead of problematic include directivesChanges Made
HtmlFormatterScriban.cs:
RenderChangeGroupcustom function for reliable template renderingmain-layout.scriban:
{{ include "change-group" section }}with{{ render_change_group section }}Test Coverage:
HtmlFormatterScribanTests.cswith 5 test scenariosTesting
All HTML formatter tests pass (5/5):
Format_WithAddedItems_IncludesAddedSectionFormat_WithRemovedItems_IncludesRemovedSectionFormat_WithModifiedItems_IncludesModifiedSectionFormat_WithBreakingChanges_IncludesBreakingSectionFormat_WithMixedChanges_IncludesAllSectionsImpact
Closes #23